-
Notifications
You must be signed in to change notification settings - Fork 855
Conversation
jenkins retest this please |
docs/multi-instance.md
Outdated
|
||
3. Update playbook (remove `es_conf_dir`, `es_data_dirs`, `es_log_dir`, `es_pid_dir` and `es_instance_name` variables) | ||
|
||
4. Deploy ansible-role |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps explicitly call out to use the new ansible role here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ansible Galaxy command to install the new version of the role is in the example below: https://github.com/elastic/ansible-elasticsearch/pull/605/files#diff-05d70e203a6d81fe483f4556abe6750dR44
Is it what you expect?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes the example is what I expect, but there is not a step in the instructions that mentions updating the role or using the new role in step 4 here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in fb64bd7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Tested process with a debian 10 box 👍 . Left one comment on a minor tweak that could be done, feel free to merge if you address it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Should fix #581